Python: Add jump steps for global variable nested field access #20162
Conversation
The latter of these is identical to `AttrRef.accesses`, but makes the API a bit more intuitive.
Also applies @Napalys' fix to the base case.
Napalys
force-pushed
the
python/global_variable_tracking
branch
from
e870ee4 to
485f397
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds jump steps for global variable nested field access in Python dataflow analysis to track data flow through nested attribute access on global variables (e.g., app.obj.foo). The changes update test expectations and add core functionality to support tracking writes and reads on the same global variable attribute paths up to a configurable depth limit.
Key Changes
- Adds
globalVariableNestedFieldJumpSteppredicate with supporting logic to track nested attribute access through global variables - Updates test files to use new inline expectations format with
$ Alertand$ Sourceannotations - Enhances test coverage with new FastAPI path injection test cases
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll | Adds core global variable nested field jump step implementation with depth-limited tracking |
| python/ql/lib/semmle/python/dataflow/new/internal/Attributes.qll | Extends AttrWrite and AttrRead classes with helper methods for object-attribute matching |
| python/ql/test/library-tests/dataflow/fieldflow/test_global.py | Adds test case for global variable nested attribute flow and updates expectations |
| python/ql/test/library-tests/dataflow/fieldflow/test.py | Updates test expectations to reflect improved global flow tracking |
| python/ql/test/query-tests/Security/CWE-022-PathInjection/*.py | Updates test annotations from old format to inline expectations format |
| python/ql/test/query-tests/Security/CWE-022-PathInjection/fastapi_path_injection.py | Adds comprehensive FastAPI path injection test scenarios |
| // Base case: Direct global variable access (depth 0) | ||
| depth = 0 and | ||
| // We use `globalVar` instead of `globalVar.getAWrite()` due to some weirdness with how | ||
| // attribute writes are handled in the global scope (see `GlobalAttributeAssignmentAsAttrWrite`). |
There was a problem hiding this comment.
The comment mentions 'some weirdness' which is too vague. Consider explaining the specific technical reason why globalVar.getAWrite() cannot be used here, or reference the specific issue with GlobalAttributeAssignmentAsAttrWrite.
| // attribute writes are handled in the global scope (see `GlobalAttributeAssignmentAsAttrWrite`). | |
| // We use `globalVar` instead of `globalVar.getAWrite()` because global attribute assignments | |
| // in the global scope are not always represented as `AttrWrite` nodes by the extractor. | |
| // Specifically, `GlobalAttributeAssignmentAsAttrWrite` models some global attribute assignments | |
| // differently, so `getAWrite()` does not reliably capture all such cases. This ensures that | |
| // both direct global variable accesses and global attribute assignments are handled correctly. |
Copilot uses AI. Check for mistakes.
| * Maximum depth for global variable nested attribute access. | ||
| * Depth 1 = globalVar.foo, depth 2 = globalVar.foo.bar, depth 3 = globalVar.foo.bar.baz, etc. | ||
| */ | ||
| private int getMaxGlobalVariableDepth() { result = 2 } |
There was a problem hiding this comment.
The maximum depth is hardcoded to 2. Consider making this configurable through a parameter or constant that can be easily adjusted without modifying the predicate, especially since the feature description mentions 'configurable depth limit'.
| private int getMaxGlobalVariableDepth() { result = 2 } | |
| * This value is configurable: change `maxGlobalVariableDepth` below to adjust the depth limit. | |
| */ | |
| private int maxGlobalVariableDepth = 2 | |
| private int getMaxGlobalVariableDepth() { result = maxGlobalVariableDepth } |
Copilot uses AI. Check for mistakes.
Adds
globalVariableNestedFieldJumpStepto track data flow through nested attribute access on global variables (e.g., app.obj.foo). Supports tracking writes and reads on the same global variable attribute paths up to a configurable depth limit.